cockroachdb cluster version fixups #6396
Conversation
iliana
changed the title
prev_cockroachdb_version
| Os::Illumos => ( | ||
| "https://oxide-cockroachdb-build.s3.us-west-2.amazonaws.com", | ||
| "tar.gz", | ||
| ), |
There was a problem hiding this comment.
The v22.1.9 artifact in this bucket is the same as the one found at the old URL, but I wanted to keep this consistent with the documentation in docs/crdb-upgrades.adoc.
| } else if settings.preserve_downgrade == version.to_string() { | ||
| // This is the expected value if the cluster was created on a | ||
| // previous version and `cluster.preserve_downgrade_option` was set. | ||
| assert_eq!(version, CockroachDbClusterVersion::POLICY); |
There was a problem hiding this comment.
This case needs to be added if, as part of doing tests, we create a cluster on an old version of CockroachDB and then set the cluster.preserve_downgrade_option setting before starting the test.
nexus/src/app/rack.rs
Outdated
| // Fill in the blueprint with the current value of the option in | ||
| // the database. | ||
| CockroachDbClusterVersion::from_str( | ||
| &cockroachdb_settings.preserve_downgrade, | ||
| ) | ||
| .map_err(|_| { | ||
| Error::internal_error(&format!( | ||
| "database has `cluster.preserve_downgrade_option` \ | ||
| set to invalid version {}", | ||
| cockroachdb_settings.preserve_downgrade | ||
| )) | ||
| })? |
There was a problem hiding this comment.
This branch should never be hit when bringing up a new control plane; this is here for the case where a CockroachDB cluster is created and cluster.preserve_downgrade_option is set before we run rack initialization as part of a test.
There was a problem hiding this comment.
(I suppose we could remove this branch for this PR and re-learn that we need it if we do a cluster upgrade...)
There was a problem hiding this comment.
I think I'd vote to keep the branch but add a comment (maybe https://github.com/oxidecomputer/omicron/pull/6396/files#r1724096463 more or less word-for-word).
| } else { | ||
| panic!( | ||
| "`cluster.preserve_downgrade_option` should not be {:?}", | ||
| settings.preserve_downgrade |
There was a problem hiding this comment.
Tiny nit - maybe include version in this panic string, so we know what it should have been?
nexus/src/app/rack.rs
Outdated
| // Fill in the blueprint with the current value of the option in | ||
| // the database. | ||
| CockroachDbClusterVersion::from_str( | ||
| &cockroachdb_settings.preserve_downgrade, | ||
| ) | ||
| .map_err(|_| { | ||
| Error::internal_error(&format!( | ||
| "database has `cluster.preserve_downgrade_option` \ | ||
| set to invalid version {}", | ||
| cockroachdb_settings.preserve_downgrade | ||
| )) | ||
| })? |
There was a problem hiding this comment.
I think I'd vote to keep the branch but add a comment (maybe https://github.com/oxidecomputer/omicron/pull/6396/files#r1724096463 more or less word-for-word).
This is a followup to #5603, which contains various fixes I needed to write when testing an actual cluster upgrade from v22.1 to v22.2.
We're currently deciding to park that plan, but I don't want these fixups to be forgotten.